deps(git): update `git-url-parse` #4635

dkhaye · 2022-07-18T20:08:01Z

What's the problem this PR addresses?
Resolves #4623

...

How did you fix it?
Upgrade to git-url-parse@^12.0.0 within @yarnpkg/plugin-git

...

Checklist

I have read the Contributing Guide.

I have set the packages that need to be released for my changes to be effective.

I will check that all automated PR checks pass before the PR gets reviewed.

➤ YN0000: │ No packages can be deduped using the highest strategy ➤ YN0000: └ Completed in 0s 264ms ➤ YN0000: ┌ Resolution step ➤ YN0002: │ @endemolshinegroup/cosmiconfig-typescript-loader@npm:3.0.2 [714f3] doesn't provide typescript (p88d18), requested by ts-node ➤ YN0002: │ @yarnpkg/gatsby@workspace:packages/gatsby doesn't provide @emotion/core (pa19bb), requested by gatsby-plugin-mdx ➤ YN0002: │ @yarnpkg/gatsby@workspace:packages/gatsby doesn't provide webpack (pe1aa2), requested by gatsby-plugin-favicon ➤ YN0060: │ @yarnpkg/monorepo@workspace:. provides eslint (pf82c3) with version 8.2.0, which doesn't satisfy what @yarnpkg/eslint-config and some of its descendants request ➤ YN0002: │ acceptance-tests@workspace:packages/acceptance-tests doesn't provide jest (pcc8ab), requested by jest-json ➤ YN0002: │ gatsby-cli@npm:3.14.0 [7243a] doesn't provide ink (p4a7fc), requested by gatsby-recipes ➤ YN0060: │ gatsby-recipes@npm:0.25.0 [bb88b] provides graphql (pc0633) with version 15.5.0, which doesn't satisfy what graphql-subscriptions requests ➤ YN0002: │ gatsby@npm:3.14.0 [118b2] doesn't provide babel-eslint (p70342), requested by eslint-config-react-app ➤ YN0002: │ react-instantsearch-dom@npm:6.6.0 [118b2] doesn't provide algoliasearch (p65251), requested by algoliasearch-helper ➤ YN0002: │ react-instantsearch-dom@npm:6.6.0 [118b2] doesn't provide algoliasearch (p5ab0d), requested by react-instantsearch-core ➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code ➤ YN0000: └ Completed in 0s 442ms ➤ YN0000: ┌ Fetch step ➤ YN0000: └ Completed in 0s 869ms ➤ YN0000: ┌ Link step ➤ YN0005: │ core-js@npm:2.6.11 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ core-js@npm:3.18.0 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ gatsby-telemetry@npm:2.14.0 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ gatsby@npm:3.14.0 [118b2] lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ core-js-pure@npm:3.21.1 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ gatsby-cli@npm:3.14.0 [7243a] lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ fsevents@patch:fsevents@npm%3A1.2.13#optional!builtin<compat/fsevents>::version=1.2.13&hash=18f3a7 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0000: └ Completed in 0s 383ms ➤ YN0000: Done with warnings in 2s 112ms

merceyz

Looks like that version is a bit buggy so I'll put this on hold for now.

mhassan1 · 2022-07-27T03:47:50Z

Based on the result of IonicaBizau/parse-url#46, it seems like git://github.com:user/repo will no longer be supported in git-url-parse@12 and higher. It seems like that could be a breaking change for Yarn users.

It seems like we are only using git-url-parse here and here for:

httpUtils.getNetworkSettings(`https://${GitUrlParse(normalizedRepoUrl).resource}`, {configuration})

Maybe we can remove our dependency on this package completely, since we are already doing URL normalization in normalizeRepoUrl.

mhassan1 · 2022-07-27T16:58:53Z

I added a couple of test cases locally in gitUtils.test.ts:

  it(`should properly normalize ${pattern} ({ git: false })`, () => {
    expect(gitUtils.normalizeRepoUrl(pattern)).toMatchSnapshot();
+   expect(GitUrlParse(gitUtils.normalizeRepoUrl(pattern)).resource).toMatchSnapshot();
  });
  it(`should properly normalize ${pattern} ({ git: true })`, () => {
    expect(gitUtils.normalizeRepoUrl(pattern, {git: true})).toMatchSnapshot();
+   expect(GitUrlParse(gitUtils.normalizeRepoUrl(pattern, {git: true})).resource).toMatchSnapshot();
  });

The snapshots are the same between git-url-parse@11 and git-url-parse@12. So at least none of the test patterns are affected by the upgrade.

mhassan1 · 2022-09-01T19:16:51Z

The reported change in behavior in git-url-parse has been closed as not a problem. It seems like Yarn users should not be affected by it.

dkhaye · 2022-09-26T17:10:28Z

Looks like that version is a bit buggy so I'll put this on hold for now.

@merceyz Can you look at this again?

As indicated by @mhassan1 here, the issue was closed as not being a problem.

For good measure, I went ahead and updated to the latest, 13.1.0.
There is another possible issue, but as indicated by @mhassan1 here this is not relevant to how this package is used within yarn.

mhassan1 · 2022-10-25T20:38:27Z

@dkhaye It looks like there are conflicts now.

dkhaye · 2022-10-25T21:12:35Z

@mhassan1 re-merged master
@merceyz PTAL

mhassan1 · 2022-11-08T14:59:31Z

@dkhaye It looks like there are conflicts again.

dkhaye · 2022-11-08T15:28:17Z

@mhassan1 Thanks again.
For posterity, my method of resolving conflicts on this branch has been:

git pull origin master
git checkout origin/master -- .yarn/cache .pnp.cjs yarn.lock
yarn install
yarn dedupe

@merceyz and @arcanis any chance this can get re-reviewed?

arcanis · 2022-11-08T15:40:59Z

Sorry for the delay, the tests pass so I think we can merge it.

Note however that as a general policy, I tend to avoid auto-updating dependencies on this repository unless they actually fix something in Yarn (under the premise that blind upgrades are just as risky if not more, as we bring untested and often unneeded new code). The issue reported in #4623 isn't attached to an actual exploitation showcase against Yarn, so upgrading git-url-parse wouldn't seem useful except for the benefit of third-party services like Snyk.

Still, I appreciate you taking time to address an open ticket!

* checkpoint * add version info * run ➤ YN0000: ┌ Deduplication step ➤ YN0000: │ No packages can be deduped using the highest strategy ➤ YN0000: └ Completed in 0s 264ms ➤ YN0000: ┌ Resolution step ➤ YN0002: │ @endemolshinegroup/cosmiconfig-typescript-loader@npm:3.0.2 [714f3] doesn't provide typescript (p88d18), requested by ts-node ➤ YN0002: │ @yarnpkg/gatsby@workspace:packages/gatsby doesn't provide @emotion/core (pa19bb), requested by gatsby-plugin-mdx ➤ YN0002: │ @yarnpkg/gatsby@workspace:packages/gatsby doesn't provide webpack (pe1aa2), requested by gatsby-plugin-favicon ➤ YN0060: │ @yarnpkg/monorepo@workspace:. provides eslint (pf82c3) with version 8.2.0, which doesn't satisfy what @yarnpkg/eslint-config and some of its descendants request ➤ YN0002: │ acceptance-tests@workspace:packages/acceptance-tests doesn't provide jest (pcc8ab), requested by jest-json ➤ YN0002: │ gatsby-cli@npm:3.14.0 [7243a] doesn't provide ink (p4a7fc), requested by gatsby-recipes ➤ YN0060: │ gatsby-recipes@npm:0.25.0 [bb88b] provides graphql (pc0633) with version 15.5.0, which doesn't satisfy what graphql-subscriptions requests ➤ YN0002: │ gatsby@npm:3.14.0 [118b2] doesn't provide babel-eslint (p70342), requested by eslint-config-react-app ➤ YN0002: │ react-instantsearch-dom@npm:6.6.0 [118b2] doesn't provide algoliasearch (p65251), requested by algoliasearch-helper ➤ YN0002: │ react-instantsearch-dom@npm:6.6.0 [118b2] doesn't provide algoliasearch (p5ab0d), requested by react-instantsearch-core ➤ YN0000: │ Some peer dependencies are incorrectly met; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code ➤ YN0000: └ Completed in 0s 442ms ➤ YN0000: ┌ Fetch step ➤ YN0000: └ Completed in 0s 869ms ➤ YN0000: ┌ Link step ➤ YN0005: │ core-js@npm:2.6.11 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ core-js@npm:3.18.0 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ gatsby-telemetry@npm:2.14.0 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ gatsby@npm:3.14.0 [118b2] lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ core-js-pure@npm:3.21.1 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ gatsby-cli@npm:3.14.0 [7243a] lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0005: │ fsevents@patch:fsevents@npm%3A1.2.13#optional!builtin<compat/fsevents>::version=1.2.13&hash=18f3a7 lists build scripts, but its build has been explicitly disabled through configuration. ➤ YN0000: └ Completed in 0s 383ms ➤ YN0000: Done with warnings in 2s 112ms * chore: change release type * fix .pnp.cjs * upgrade git-url-parse to 13.1.0 * add missing .yarn/ files * run yarn dedupe * run 'yarn dedupe' Co-authored-by: merceyz <merceyz@users.noreply.github.com>

dkhaye added 3 commits July 18, 2022 13:45

checkpoint

6ce50fb

add version info

5196afd

dkhaye changed the title ~~Upgrade parse-path to avoid Authorization Bypass vulnerability~~ fix(plugin-git) Upgrade parse-path to avoid Authorization Bypass vulnerability Jul 18, 2022

dkhaye changed the title ~~fix(plugin-git) Upgrade parse-path to avoid Authorization Bypass vulnerability~~ fix(plugin-git): Upgrade parse-path to avoid Authorization Bypass vulnerability Jul 18, 2022

merceyz changed the title ~~fix(plugin-git): Upgrade parse-path to avoid Authorization Bypass vulnerability~~ deps(git): update git-url-parse Jul 18, 2022

chore: change release type

14a0a04

merceyz requested changes Jul 18, 2022

View reviewed changes

arcanis requested a review from merceyz August 4, 2022 11:58

dkhaye added 4 commits September 26, 2022 11:30

merge develop

bba139e

fix .pnp.cjs

73b934c

upgrade git-url-parse to 13.1.0

3ba6b93

add missing .yarn/ files

f0f0ca6

merge master

d411ed2

run yarn dedupe

40966b7

dkhaye added 2 commits November 8, 2022 10:18

merge develop (again)

3fe839c

run 'yarn dedupe'

75de9b0

arcanis merged commit 8171d09 into yarnpkg:master Nov 8, 2022

legobeat mentioned this pull request Jul 6, 2024

3.x fix import fallback #6385

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps(git): update `git-url-parse` #4635

deps(git): update `git-url-parse` #4635

dkhaye commented Jul 18, 2022

merceyz left a comment

mhassan1 commented Jul 27, 2022 •

edited

Loading

mhassan1 commented Jul 27, 2022 •

edited

Loading

mhassan1 commented Sep 1, 2022

dkhaye commented Sep 26, 2022 •

edited

Loading

mhassan1 commented Oct 25, 2022

dkhaye commented Oct 25, 2022

mhassan1 commented Nov 8, 2022

dkhaye commented Nov 8, 2022

arcanis commented Nov 8, 2022

deps(git): update git-url-parse #4635

deps(git): update git-url-parse #4635

Conversation

dkhaye commented Jul 18, 2022

merceyz left a comment

Choose a reason for hiding this comment

mhassan1 commented Jul 27, 2022 • edited Loading

mhassan1 commented Jul 27, 2022 • edited Loading

mhassan1 commented Sep 1, 2022

dkhaye commented Sep 26, 2022 • edited Loading

mhassan1 commented Oct 25, 2022

dkhaye commented Oct 25, 2022

mhassan1 commented Nov 8, 2022

dkhaye commented Nov 8, 2022

arcanis commented Nov 8, 2022

deps(git): update `git-url-parse` #4635

deps(git): update `git-url-parse` #4635

mhassan1 commented Jul 27, 2022 •

edited

Loading

mhassan1 commented Jul 27, 2022 •

edited

Loading

dkhaye commented Sep 26, 2022 •

edited

Loading